fix(ci): speed up tall-prompt Storybook setup - #5492
Conversation
liuxiaocs7
changed the title
liuxiaocs7
changed the title
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed at 4557a4ce0. The remaining code change is solid — execCommand('insertText') exercises the real beforeinput→Astryx→onChange→saveCurrentDraft path, Shift+Enter/submission stay on real keydown, and the assertions are stronger (full-draft innerText check). PNG removals are unreferenced. No code blockers.
P2 — the title/body no longer describe this diff. Two of the three claimed changes dissolved on rebase: the 2,000→4,000 measure-ahead margin already landed via #5499 (and #5499's justification is more complete — 6,000 triggers traversal-story per-step drift), and the WorkHub navigation-highlight wait was removed along with the Work rail in #5622. The squash title stabilize Storybook setup and transcript measurements would enter main history with no transcript-measurement change in it. Suggest retitling to the actual scope (tall-prompt insertion + PNG cleanup + plugin license headers) and noting on #5490 which parts landed where.
P3 inline — undeclared scope: ASF headers on scripts/plugins/codex-app-server-executor/*. Benign and self-consistent, but /scripts/plugins is export-ignore and the header check already passed without them — disclose in the body or split out.
Prepare prompt lines with native text insertion while preserving the real Shift+Enter and submit paths. Assert the complete draft before measuring scroll reversal, so an incomplete fixture cannot pass the geometry check. Remove the four temporary module-page before/after PNGs from docs/images/pr. Fixes apache#5490 Generated-by: Codex
Add canonical ASF headers to the README and composition file, and place the fixture header directly below its shebang. The example remains excluded from source archives; keep this cleanup separate from the Storybook timing fix. Generated-by: Codex
4557a4c to
9093831
Compare
liuxiaocs7
changed the title
|
@Astro-Han Thanks for the review. The title is now fix(ci): speed up tall-prompt Storybook setup, and the body describes the actual diff: native prompt insertion, PNG cleanup, and the separately committed plugin license headers. The description records that the transcript margin landed in #5499 and the WorkHub navigation check was removed in #5622. I also added that breakdown to #5490. The header cleanup is isolated in Rebased onto Posted by Codex on behalf of @liuxiaocs7. |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Delta review at 9093831fc (was 4557a4ce0): rebase + history rewrite, effective diff byte-identical to the previously-reviewed patch. Prior findings resolved — the retitle to 'speed up tall-prompt Storybook setup' now names the dominant change; the body explicitly declares all three parts and correctly attributes the margin change to #5499 and the nav-highlight assertion removal to #5622; the plugin license headers are now a separate, honestly-titled commit with export-ignore context disclosed. The insertion fix still exercises the real beforeinput→draft path and PNG deletions remain unreferenced. No new findings.
|
Thanks @Astro-Han for reviewing! |
2 participants
Summary
Preparing the 80-line prompt fixture through thousands of synthetic keystrokes can consume Storybook's 15-second deadline before submission and scroll assertions run. Insert each line natively, keep Shift+Enter and submission on the real keyboard path, and assert the complete draft before checking scrolling.
/scripts/pluginsis excluded from source archives throughexport-ignore.Fixes #5490. Its transcript measure-ahead change already landed in #5499 (2,000 to 4,000 px), and the WorkHub navigation-highlight check was removed with the Work rail in #5622.
Verification
After rebasing onto
c299cc17a:git diff --checkand the protocol epoch guard againstmainpassed.AI use
Tool(s) and scope: Codex prepared the Storybook fix and cleanup, rebased the branch, updated the PR description, and ran validation.
Checklist
Does this PR entail a change in behavior?